Upgrade pr-finisher skill with agent-merge patterns#37905
Merged
Conversation
Adopts agent-merge concepts adapted for the pre-merge scope: - Three-condition framing (Reviews/Checks/Mergeable) worked concurrently - Mergeable condition (CONFLICTING, BEHIND) now explicit - CI anti-pattern guardrails (no disabling shared tooling, no temp disables, no workaround+fix bundling, anti-pattern test) - Root-cause framing with gh run view --log-failed - Never block on CI (no sleep, no --watch, no gh run watch) - GH_PAGER='' required for non-interactive shells - No stand-alone PR comments; reply-only - Structured stopping-point summary with status emoji vocabulary - Tooling drift scan after BEHIND branch updates - Hard rule: never merge (gh pr merge forbidden) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the pr-finisher skill documentation to use a three-condition “merge-ready” framework (Reviews / Checks / Mergeable) with explicit guardrails (notably: never merge) and a structured stop/summarize flow, aligned with patterns from agent-merge but scoped to pre-merge preparation.
Changes:
- Reframes the skill around concurrently satisfying Reviews, Checks, and Mergeable conditions (including
CONFLICTING/BEHINDhandling). - Adds hard rules and CI anti-pattern guidance (no waiting/polling CI, no disabling checks/tooling, require root-cause investigation).
- Introduces a structured stopping-point summary template with bounded status vocabulary (✅/❌/⏳/❓).
Show a summary per file
| File | Description |
|---|---|
| .github/skills/pr-finisher/SKILL.md | Rewrites the skill guide to adopt the three-condition merge-ready model, guardrails, and a structured workflow/summary format. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 3
| # PR Finisher | ||
|
|
||
| Use this skill when asked to get a pull request to a merge-ready state. | ||
| Drive an open PR for the current branch to a merge-ready state. **Do not merge.** When all conditions are satisfied, report ready-for-human-merge and stop. |
| - Use GitHub tools to inspect the current pull request status and check runs. | ||
| - When the user mentions CI, build, test, or workflow failures, fetch the failing job logs before deciding on a fix. | ||
| - Distinguish between failures already covered by local commands (`fmt`, `lint`, `test-unit`, `test`) and unrelated external failures. | ||
| - **Do not merge.** Never run `gh pr merge`, enable auto-merge, or enqueue. This skill stops at "ready for merge." |
| 8. If wasm golden tests fail, or a test fix changes the expected wasm compiler output, run `make update-wasm-golden`. | ||
| 9. Re-run the affected local validation until it passes. | ||
| 10. Push the final changes. | ||
| Top-level PR comments and review bodies are useful feedback but are **not** a merge gate (GitHub has no resolve state for them). Read and action useful ones; do not block on them. |
The skill runs inside a cloud agent whose pushes do not trigger CI. Local make targets are the only authoritative correctness signal before push; CI is observational and goes stale at the moment the agent commits. Re-running CI is a hand-off to a human. - New Execution context section spelling out the constraints - Checks split into local (gate) vs prior CI (root-cause only) - Removed post-push re-check loop and 'Only Checks pending' wait state; agent runs once and stops - Summary adds explicit 'Hand-off: CI must be re-triggered' line - Stronger 'reproduce locally where possible' for CI fixes; flag not-locally-reproducible failures explicitly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (27161179157)
|
Contributor
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrade pr-finisher skill with agent-merge patterns
Summary
Rewrites
.github/skills/pr-finisher/SKILL.mdto make the skill accurate and safe for use inside a GitHub Copilot cloud agent — an execution context where the agent's own pushes do not trigger CI, making the old "wait for checks" pattern actively harmful. Two commits land the change: the first introduces agent-merge structural patterns; the second reframes the entire skill around cloud-agent constraints.What changed
File:
.github/skills/pr-finisher/SKILL.mdNet diff: +183 / −100 lines across two commits.
New: Execution context section
Explains the cloud-agent constraints that govern every decision in the skill:
statusCheckRollupgoes stale the moment the agent commits.maketargets are the only authoritative correctness signal before push.sleep, no--watch, no re-check loops; one pass, then stop.New: Three-condition merge-ready model (concurrent)
Replaces an implicit ad-hoc checklist with an explicit table of three conditions worked in parallel:
copilot-review+ GraphQLreviewThreadsmake fmt/lint/test-unit/testgreen; prior CI failures root-caused at log levelmergeable: MERGEABLE, notBEHINDwhere up-to-date is requiredNew: Mergeable condition handling
CONFLICTING→ resolve using repo conventions;ask_userif resolution is ambiguous.mergeStateStatus: BEHIND→ update branch from base, then scan for tooling drift (lockfiles, toolchains, lint configs) and flag any drift in the summary.New: CI-fix anti-pattern guardrails
Hard-forbidden actions the agent must never take:
Adds an explicit anti-pattern test: if the change would make the failure invisible on future PRs without solving it, stop and escalate via
ask_user.Changed: Local vs prior-CI split for Checks
makerun = gate; must be green before any push.gh run view --log-failed; fixes reproduced locally where possible; not-locally-reproducible failures flagged explicitly in the summary.gh pr checksre-check loop;Only Checks pendingwait state.New: Hard rules block
Consolidated, strengthened, and promoted to a top-level section:
gh pr mergeor enable auto-merge.ghwithGH_PAGER=""for non-interactive shells.New: Structured summary format
Required stopping-point output using ✅ / ❌ / ⏳ / ❓ status vocabulary, with:
Hand-off: CI must be re-triggered by a maintainerline.Still needed:field for anything requiring human action.New: Completion standard
An explicit multi-point checklist defining when the task is truly done (local targets, review threads, mergeable, CI inspection, single push, summary printed, no merge run).
Impact
Checklist for reviewers
Hand-offwording matches how maintainers actually re-trigger CI (close/reopen,workflow_dispatch, or push).GH_PAGER=""requirement is consistent with how other skills invokegh.